Update mysqld_exporter to v0.16.0 #1643
Conversation
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
5 times, most recently
from
152acc0
to
b7b1fb2
Compare
internal/component/loki/source/aws_firehose/internal/handler_test.go
Outdated
Show resolved
Hide resolved
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
from
b7b1fb2
to
976d501
Compare
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
2 times, most recently
from
043860e
to
0f466b4
Compare
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
from
0f466b4
to
40b6e18
Compare
|
This PR has not had any activity in the past 30 days, so the |
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
4 times, most recently
from
9697e59
to
bb8e29b
Compare
go.mod
Outdated
| @@ -161,7 +161,7 @@ require ( | |||
| github.com/prometheus/common/sigv4 v0.1.0 | |||
| github.com/prometheus/consul_exporter v0.8.0 | |||
| github.com/prometheus/memcached_exporter v0.13.0 | |||
| github.com/prometheus/mysqld_exporter v0.14.0 | |||
| github.com/prometheus/mysqld_exporter v0.15.1 | |||
There was a problem hiding this comment.
Any new config options that need to be exposed since it's a minor version, it can have new features?
From grafana/mysqld_exporter branch `refactor_collector`, including changes in prometheus/mysqld_exporter#774.
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
from
bb8e29b
to
20cb1b5
Compare
| @@ -122,7 +123,7 @@ func New(log log.Logger, c *Config) (integrations.Integration, error) { | |||
| } | |||
|
|
|||
| scrapers := GetScrapers(c) | |||
| exporter := collector.New(context.Background(), string(dsn), scrapers, log, collector.Config{ | |||
| exporter := collector.New(context.Background(), string(dsn), scrapers, slog.Default(), collector.Config{ | |||
There was a problem hiding this comment.
@thampiotr New here now requires an slog.Logger, is there any adapter in the Alloy codebase?
cristiangreco
force-pushed
the
cristian/mysqld-exporter-upd
branch
from
962ed3c
to
69337c6
Compare
cristiangreco
changed the title
| @@ -122,7 +124,8 @@ func New(log log.Logger, c *Config) (integrations.Integration, error) { | |||
| } | |||
|
|
|||
| scrapers := GetScrapers(c) | |||
| exporter := collector.New(context.Background(), string(dsn), scrapers, log, collector.Config{ | |||
| logger := slog.New(slogadapter.GoKitHandler{Logger: log, Group: ""}) | |||
There was a problem hiding this comment.
I think that you could just cast the logger into our internal logger type log.(*logging.Logger) and then call the Handler() function to build the slog logger: slog.New(l.Handler())
There was a problem hiding this comment.
in this case, the slogadapter would not be needed
There was a problem hiding this comment.
Just tried that:
panic: interface conversion: log.Logger is *log.context, not *logging.Logger
Unfortunately it seems it's a go-kit logger rather than the internal logging.Logger.
There was a problem hiding this comment.
If we could swap the go-kit logger for the internal logging.Logger that would indeed be much easier. I'm not sure how much work is that though.
There was a problem hiding this comment.
Ah I see, it's because of the log.With here: https://github.com/grafana/alloy/blob/main/internal/runtime/internal/controller/node_builtin_component.go#L172
There was a problem hiding this comment.
this is similar to the problem that @dehaansa encountered in this support bundle PR
There are a few options:
- use the adapter that you made
- implement log.With in the logging.Logger to pass the logging.logger instead of the go-kit logger
- pass a slog logger instead of a gokit logger
There was a problem hiding this comment.
I stole the adapter in this PR from @aknuds1 's work at https://github.com/grafana/mimir/pull/9844/files#diff-36917e58779c0933db5147cf951d9c445a912ba808cf35c32ac951063b91f93f
Option 2 or option 3 that you mentioned above are more appealing though, as we would avoid relying on an adapter forever.
There was a problem hiding this comment.
yeah the thing is that we want to migrate the code base to use slog everywhere. The idea behind the slogadapter that we already have is to be able to use slog in the areas where go-kit is still needed. It feels like the adapter that this PR is adding is going the wrong way, it's going from slog to go-kit and back to slog.
This is out of the scope of this PR so if that's ok I would suggest to pause it for now and that we find a better solution to pass the slog logger properly to the components that can use it directly
There was a problem hiding this comment.
when do you need the update of the exporter in Alloy?
There was a problem hiding this comment.
Can we add a test for this?
PR Description
From grafana/mysqld_exporter branch
refactor_collector, including changes in prometheus/mysqld_exporter#774.Which issue(s) this PR fixes
n.a.
Notes to the Reviewer
n.a.
PR Checklist